Conversation
…ng-cpp @codemirror/lang-markdown
npm install @lezer/lr npm install --save-dev @lezer/generator
|
A few more commits, to fix issues accumulated during migration:
|
|
After 1.5 cups of coffee I can say it looks pretty good! Some comments: I like the new breakpoint marker, and having a visual indicator when you Run To Line. Maybe the breakpoint and current PC gutters could be adjacent for a bigger hit target? And maybe a cursor change or highlight when you move the pointer over the gutter. I noticed that the gutters expand and push the editor to the right after the first compilation completes. Actually, the gutters expand when you scroll down into a region that has gutter elements. It's a little distracting on first load but I can't think of an easy solution, that seems to just be the way CM6 works. I wish there was a way to make error tooltips show up on pointer move, but I don't think CM6 has much to do with this, since the marker just uses the title attribute. The Asset Editor is a complete mess but it works by modifying text in the editor directly via project.updateFile(). It also expects projectWindows.undoStep() to undo the last updateFile() operation, because CM doesn't have a global undo history. At least I think that's how it works ... I see this is broken in the CM5 version also, so I need to investigate. I still want to test on mobile devices and look at performance, but I hope to merge this soon! Nice job! |
For both I should have something shortly.
Agreed that some improvement is needed there. Having the CurrentPC gutter be clickable was a bit of a temporary implementation as I was imaging that the user would focus on toggle (multiple) breakpoints. There's also a question of how to treat error marker, current PC marker and other info markers. Question being which of these can/should share the same physical gutter column. Does a priority system make sense? Want to avoid taking up too much horizontal space, while also avoiding a confusing experience. The approach I've been taking for the migration is a) get all the functionality working, using separate gutter columns to keep things simple, b) once that's all settled, and you're happy with it, revisit what you want to CM6 experience to be. A key enhancement would be multiple breakpoints, assuming the different runtimes can easily support them.
Yep, this needs to be fixed. This should be addressable through Are 3 bytes (opcode + operand) always enough? And, still okay with the gutter surpressing bytes? For example, https://8bitworkshop.com/v3.12.1/?platform=c64&file=hello.dasm contains Oh and variable highlights: currently they show up on an extra line. In the production version you can copy/paste that value. In the CM6 version you currently can't. I think probably we can fix this, but I wonder if you think of this as an important feature or just a random useless thing that happens to be possible. Have variables values show up as tooltips might be a more familiar way to handle things. That should be easy. In editors.ts, try setting
I think this is possible via the gutter's
I noticed that undo in the asset editor (in both CM5 and CM6 version) lets mod+z undo the underlying file (i.e. the running program would revert to the old sprites), but not undo the asset visualization. The CM6 undo history is quite capable and has all sorts of capabilities in terms of supporting undo barriers, and having ways to have multiple changes be combined. All that functionality does come we some requirements to call the APIs correctly. (Sometimes naive attempts appear to work at first, but prove flaky.) Also, I was thrown for a loop for a bit when CMD-Z was all wonky. Turned out not having
|
- state fields use RangeSet<GutterMarker> instead of simple maps/sets - lineMarker / lineMarkerChange replaced with gutter() configs and markers - implement eq() in markers to avoid unnecessary churn - BREAKPOINT_MARKER CURRENT_PC_MARKER reusable across all lines - removed the unsused shownErrorLinesField
Add gutter `initialSpacer`s
value now renders after the end of the line, instead of breaking it up
Made improvements to gutter around performance, and leaning into CM6 approach.
I had the error gutter in between the breakpoint gutter and currentpc gutter, which made finding the right place to click feel bad. I merged error and breakpoint gutter (breakpoints hide while there are errors). Potential breakpoints now appear on hover as red dot with 50% opacity. TODO: consider whether hover and setting of breakpoints is limited to lines that have an offset (i.e. valid breakpoint)
Should be fixed. Initial space bytes is hardcoded to TODO I did not address display of more than three bytes, so the current behavior of just showing the first three bytes is untouched. To maintain consistent status column width, I used "ⓧ" as the initialSpacer, since even with a monospace font, "ⓧ" is wider than "●".
The native browser tooltip does show up, but it's only after 1000ms or so. I also thought it was not showing, so I'm clearly not patient enough. A custom styled tooltip that appears immediately is possible. Unrelated, filed #212 (the issue is more easily recreated in CodeMirror 6) as a TODO. |
update runToPC so that mem_info is hidden and currentpc is reset
|
Implemented multiple breakpoints, except for MAME In trying to implement MAME… TBH, I can't tell if MAME breakpoints are entirely broken (both CM5 and CM6 versions). Whether I single step or set a specific breakpoint, MAME rarely stops where I ask it to. It does run for a bit and stop somewhere. I can't identify a I tried Not sure about the right docs, but I'm guessing https://docs.mamedev.org/debugger/execution.html is relevant. Thoughts? |
|
Other than the couple of TODO I mentioned, I think it's all looking pretty good. Let me know if you have a short list of "must fix before accepting the PR" and I/we can focus on that. |
The clickable breakpoint gutter is not too bad now that it has the cursor affordance. And it's cool how you implemented breakpoints with runToPC()! Even though the breakpoints are useful I am finding that I miss the old click-the-offset-run-to-PC method which was good for quickly iterating through loops. Can they co-exist? I wonder if we can lose line numbers to free up gutter space? I like the aesthetics, but I don't know if they're useful since we have error markers.
3 bytes is enough for 6502 instructions, the Z80 sometimes uses 4 bytes but the spaces are removed so they take up the same number of chars. I like the ellipsis idea, I can't guarantee the assembler listing files are parsed properly so that it'll always work -- sometimes it splits across multiple lines or just omits bytes.
I don't think that's a big deal. It'd be nice to show live variables in a tooltip or overlay or something, there's no type information though so you have to guess what format to display. I end up using the Memory Browser for that kind of thing. |
I was never able to get MAME debugging working properly, apparently it goes into a separate event loop when debugging and that causes all kinds of problems. I hear someone got it working by using an Emscripten option that invokes the Relooper algorithm or something, but I ran out of patience with it. I was considering removing debugging support from MAME altogether. |
New `node_modules` target, ensures `npm install` has run before attempting to build grammars with `./node_modules/.bin/lezer-generator`
New `submodules` target that ensures `git submodule update --init --recursive` runs before `buildtsc`
Avoids spurious error messages when `tsweb` is run after fresh checkout by depending on `submodules` and `buildgrammars`. This ensures both are fully finished before spawn several child processes that will race each other.
|
Upgrade to CodeMirror6, supports multi-cursor editing, and other modern conveniences.
Removed CodeMirror5 implementation:
codemirrorsubmodulesrc/codemirror/*css/codemirror.cssInstalled CodeMirror6 npm packages:
@codemirror/commands@codemirror/view@codemirror/lang-cpp@codemirror/lang-markdownFiled changed:
src/ide/views/editors.tsSignificant changes; bulk of migration effortcss/ui.cssRemoved no longer used stylesNew files:
src/ide/views/filters.tsText transform filter (for BASIC uppercase support)src/ide/views/gutter.tsGutter decorations and widgetssrc/ide/views/visuals.tsMain editor decorations and widgetssrc/parser/lang-*.tsStreamParser based languages, based on original CM5 implementationssrc/themes/mbo.tsmigratedmbothemesrc/themes/cobalt.tsmigratedcobaltthemesrc/themes/editorTheme.tsCSS for main editorsrc/themes/disassemblyTheme.tsCSS for disassembly viewMigration notes:
src/codemirror/ecs.jsRemoved, but not migrated; appears unusedsrc/codemirror/vasm.jsRemoved, but not migrated; appears unused@codemirror/lang-javascriptNot installed; javascript platform appears unusedTODO
src/parser/lang-*.tsstream parsers may require a few final color / style tweakssrc/themes/mbo.tsandsrc/themes/cobalt.tsthemes may require a few final color / style tweakssrc/parser/lang-*.tsstream parser to Lezer parsers for languages were this is reasonable